Skip to content

fix(robot): handle empty permissions in robot view - #998

Merged
bupd merged 1 commit into
goharbor:mainfrom
Jay2006sawant:fix/robot-view-empty-permissions
Aug 4, 2026
Merged

fix(robot): handle empty permissions in robot view#998
bupd merged 1 commit into
goharbor:mainfrom
Jay2006sawant:fix/robot-view-empty-permissions

Conversation

@Jay2006sawant

@Jay2006sawant Jay2006sawant commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Description

harbor robot view panics when the Harbor API returns a robot with an empty or nil permissions array. The view indexed robot.Permissions[0] unconditionally, while harbor robot list already guarded this case.

This PR adds safe fallbacks for the permission count and project namespace header, matching the list view behavior.

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation update
  • Chore / maintenance

Changes

  • Add robotPermissionSummary helper to safely read permission count and namespace
  • Use placeholders (0, --) when permission details are missing
  • Add unit tests for nil/empty and populated permissions

Test plan

  • go test ./pkg/views/robot/view/... -v
  • Verified robotPermissionSummary returns safe defaults for nil/empty permissions (no index panic)

@Jay2006sawant
fix(robot): handle empty permissions in robot view
760570b 
Fixes goharbor#997

Signed-off-by: Jay2006sawant <jay242902@gmail.com>
@Jay2006sawant

Jay2006sawant commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@Vad1mo Can you please look into this?

NucleoFusion
NucleoFusion reviewed Jun 20, 2026
View reviewed changes

@NucleoFusion NucleoFusion left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think so we can create robots with 0 permissions. (I confirmed you cant)
But i think the gaurd is not a bad to have.
@qcserestipy what do you think? Should we have it or is it redundant?

@codecov

codecov Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 62.50000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 9.19%. Comparing base (60ad0bd) to head (760570b).
⚠️ Report is 196 commits behind head on main.

Files with missing lines Patch % Lines
pkg/views/robot/view/view.go 62.50% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff            @@
##             main    #998      +/-   ##
=========================================
- Coverage   10.99%   9.19%   -1.80%     
=========================================
  Files         173     321     +148     
  Lines        8671   16088    +7417     
=========================================
+ Hits          953    1480     +527     
- Misses       7612   14474    +6862     
- Partials      106     134      +28

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@qcserestipy

qcserestipy commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

@NucleoFusion I think we could get this into the code. I would be curious to know how this issue was found if the cli and ui both do not allow to create robots with 0 permissions in practice @Jay2006sawant.

@qcserestipy qcserestipy added status/needs-feedback Waiting for input from the issue author or reporter. May be closed after 7 days. enhancement New feature or request status/in-progress Work on this issue has started or a linked pull request is actively being developed. labels Jun 30, 2026
@qcserestipy
qcserestipy self-requested a review July 6, 2026 07:21
qcserestipy
qcserestipy approved these changes Jul 20, 2026
View reviewed changes

@qcserestipy qcserestipy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! i think the guard is worth keeping. robot list already guards this the same way (pkg/views/robot/list/view.go), so this just brings the view in line with it. and even if the ui doesnt let you create a robot with 0 permissions, this is api response data, so a view shouldnt index Permissions[0] unconditionally anyway. cheap check, avoids a panic. lgtm.

@qcserestipy qcserestipy added status/has-approved-pr Issue has a linked pull request that has been approved and is waiting to be merged. and removed status/needs-feedback Waiting for input from the issue author or reporter. May be closed after 7 days. status/in-progress Work on this issue has started or a linked pull request is actively being developed. labels Jul 20, 2026
bupd
bupd approved these changes Aug 4, 2026
View reviewed changes

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@bupd
bupd merged commit 1b69c58 into goharbor:main Aug 4, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qcserestipy qcserestipy qcserestipy approved these changes
@bupd bupd bupd approved these changes
+1 more reviewer
@NucleoFusion NucleoFusion NucleoFusion left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement New feature or request status/has-approved-pr Issue has a linked pull request that has been approved and is waiting to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[bug]: robot view panics when robot has empty permissions

4 participants

@Jay2006sawant @qcserestipy @bupd @NucleoFusion